-
Notifications
You must be signed in to change notification settings - Fork 59
Use Postgres for Grafana backend #2494
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
| const project = gcp.organizations.getProjectOutput({}); | ||
|
|
||
| // use existing default network (needs to have a private vpc connection) | ||
| export const privateNetwork = gcp.compute.Network.get( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In infra stack, we're already creating this resource, so when this is imported from infra stack, it triggers a duplicate resource error. AFAICT, all we need from here is the ID, so no real point in even creating this resource.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sounds plausible
| "@pulumiverse/grafana": "0.16.3", | ||
| "@lfdecentralizedtrust/splice-pulumi-common": "1.0.0" | ||
| "@lfdecentralizedtrust/splice-pulumi-common": "1.0.0", | ||
| "@pulumi/gcp": "8.32.1" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not 100% sure why having this in common did not suffice, but it didn't...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
npm 🤷
| { | ||
| dependsOn, | ||
| aliases: [ | ||
| { name: 'observabilty' }, // Legacy typo |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Damn, this one took me a while to figure out!
Signed-off-by: Itai Segall <itai.segall@digitalasset.com>
|
/cluster_test |
|
Deploy cluster test triggered for Commit f760fab2809d369438ee81e87b16c06048b2669e in , please contact a Contributor to approve it in CircleCI: https://app.circleci.com/pipelines/github/DACH-NY/canton-network-internal/34495 |
|
@nicu-da if approved, please feel free to merge this and bump CILR to apply there |
moritzkiefer-da
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice thank you!
build-tools/cncluster
Outdated
| # are currently deployed | ||
| # grep exits with return code 1 if it finds no matches so we account for that | ||
| query_result=$(kubectl get StatefulSets -A -o json 2> /dev/null | jq '.items[].spec.template.spec.containers[].image' | { grep -v prometheus || test $? = 1; }) | ||
| query_result=$(kubectl get StatefulSets -A -o json 2> /dev/null | jq '.items[].spec.template.spec.containers[].image' | grep -v postgres | { grep -v prometheus || test $? = 1; }) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is filtering out postgres ok? Isn't this one of the main stateful sets we have? It seems like we really should just filter out everything in the observability namespace no?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
will push a fix for that
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pushed
| const project = gcp.organizations.getProjectOutput({}); | ||
|
|
||
| // use existing default network (needs to have a private vpc connection) | ||
| export const privateNetwork = gcp.compute.Network.get( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sounds plausible
| "@pulumiverse/grafana": "0.16.3", | ||
| "@lfdecentralizedtrust/splice-pulumi-common": "1.0.0" | ||
| "@lfdecentralizedtrust/splice-pulumi-common": "1.0.0", | ||
| "@pulumi/gcp": "8.32.1" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
npm 🤷
[static] Signed-off-by: Moritz Kiefer <moritz.kiefer@purelyfunctional.org>
Tested manually by running infra up on a scratchnet and seeing grafana working afterwards, and seemingly happy with the postgres config:

(and when I got things wrong in how it's configured, it definitely was not happy)
cluster test for sanity that I didn't break anything
(this preflight also applied the change on scratchB, where I did not apply it manually before, and I confirmed after the run that it uses postgres there too, and seems to be working as expected)
Pull Request Checklist
Cluster Testing
/cluster_teston this PR to request it, and ping someone with access to the DA-internal system to approve it./hdm_teston this PR to request it, and ping someone with access to the DA-internal system to approve it.PR Guidelines
Fixes #n, and mention issues worked on using#nMerge Guidelines